feat: thread encryptionVersion in SDK by bdesoky · Pull Request #8955 · BitGo/BitGoJS

bdesoky · 2026-06-05T15:59:55Z

This pull request introduces support for selecting an encryption version (either v1 or v2) across various wallet operations, including key encryption, wallet sharing, QR data generation, and secret splitting. The changes add an optional encryptionVersion parameter to relevant APIs, types, and internal calls, ensuring that encryption can be consistently upgraded or specified throughout the codebase. Comprehensive tests are added to verify correct behavior for both v1 and v2 encryption envelopes.

Encryption Version Support Across Modules

Added optional encryptionVersion parameter to key encryption functions and types in sdk-api, key-card, passkey-crypto, and express modules, allowing callers to specify which encryption version to use. [1] [2] [3] [4] [5]
Updated all internal calls to encryptAsync and related encryption operations to pass through the encryptionVersion parameter where provided. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]

API and Type Updates

Extended API request/response codecs and parameter interfaces (e.g., UpdateLightningWalletClientRequest, GenerateQrDataCoinParams, SplitSecretOptions) to include the encryptionVersion field. [1] [2] [3]
Propagated encryptionVersion through wallet update, sharing, and keychain creation flows. [1] [2] [3] [4]

QR Data Generation Enhancements

Updated QR data generation functions to accept and utilize encryptionVersion, ensuring that the passphrase envelope (Box D) is created with the correct version. [1] [2] [3]

Testing and Validation

Added and expanded unit tests to verify that QR data generation produces the correct envelope version depending on the encryptionVersion parameter, and that Box D is omitted when required fields are missing. [1] [2]

Dependency and Import Updates

Updated imports to include the new EncryptionVersion type where needed. [1] [2] [3] [4]

These changes collectively ensure that encryption versioning is handled robustly and consistently throughout the codebase, improving security flexibility and future-proofing wallet operations.

linear-code · 2026-06-05T15:59:59Z

WCN-774

bdesoky · 2026-06-05T16:30:29Z

@claude

github-actions · 2026-06-05T16:30:50Z

Claude finished @bdesoky's task —— View job

Code Review for PR #8955: feat: thread encryptionVersion in SDK

Todo List

Analyze PR changes and understand encryption version implementation
Review key files for code quality and consistency
Check test coverage and implementation
Verify security considerations
Provide comprehensive code review feedback

Summary

This PR successfully implements encryption version threading throughout the BitGoJS SDK, adding support for v2 encryption envelopes while maintaining backward compatibility. The implementation is thorough and well-tested.

Strengths

✅ Comprehensive Implementation

Correctly threads encryptionVersion parameter through 27 files across multiple modules
Maintains backward compatibility by making the parameter optional everywhere
Follows consistent patterns across all affected modules

✅ Excellent Test Coverage

Added comprehensive unit tests for QR data generation with different encryption versions
Created regression tests for TSS keychain creation (ecdsaEncryptionVersion.ts)
Added wallet-level tests for encryption version forwarding (walletsEncryptionVersion.ts)
Tests verify both v1 and v2 envelope creation and proper parameter forwarding

✅ Security Best Practices

Uses async encryption methods (encryptAsync, decryptAsync) for v2 support
Maintains separation between sync v1 and async v2 encryption paths
Proper error handling in test cases

✅ Code Quality

Clean, consistent API design with optional parameters
Good documentation in function signatures
Follows existing codebase patterns and conventions

Areas for Improvement

🔍 Minor Concerns

Type Safety Enhancement (modules/sdk-api/src/types.ts:229)
```
encryptionVersion?: EncryptionVersion;
```
Consider adding JSDoc documentation to clarify when this parameter should be used.
Async Function Naming (modules/key-card/src/generateQrData.ts:214)
The generateQrDataAsync function could benefit from more explicit documentation about when to use it vs the sync version.
Test Isolation
Some tests could benefit from more explicit setup/teardown to ensure test isolation, particularly in the wallet sharing tests.

Technical Assessment

Architecture: The threading approach is well-designed, using optional parameters consistently and providing both sync and async variants where needed.

Backward Compatibility: Excellent - all changes are additive and maintain existing behavior when encryptionVersion is not specified.

Performance: No performance concerns - async functions are used appropriately for v2 encryption.

Security: The implementation correctly uses the newer Argon2id-based v2 encryption when requested while maintaining v1 SJCL compatibility.

Files with Excellent Implementation

modules/key-card/src/generateQrData.ts - Clean separation of sync/async variants
modules/sdk-core/src/bitgo/wallet/wallet.ts - Proper parameter forwarding in sharing methods
Test files show thorough coverage of edge cases and regression scenarios

Recommendation

✅ APPROVE - This is a well-implemented feature with excellent test coverage and proper security considerations. The changes successfully enable v2 encryption support while maintaining full backward compatibility.

The code is production-ready and follows BitGo's development standards. The comprehensive test suite provides confidence in the implementation's correctness.

bdesoky · 2026-06-05T16:53:51Z

+      encryptedRShare = await this.bitgo.encryptAsync({
+        input: stringifiedRShare,
+        password: params.walletPassphrase,
+        encryptionVersion: 1,


hardcoding the version to 1 here since v2 would have gone through the useV2 branch above

bdesoky · 2026-06-05T16:54:01Z

      input: userGpgKey.privateKey,
      password: walletPassphrase,
      adata: `${EddsaMPCv2Utils.MPS_DSG_SIGNING_USER_GPG_KEY}:${adata}`,
+      encryptionVersion: 1,


same as above

bdesoky · 2026-06-05T17:48:02Z

-  const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion: 2 });
+  const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion });


there are no callers of this methods after a github search

hmm you wanna check with @mohammadalfaiyazbitgo on this one?

sure can do that

vinhkhangtieu

add encryptionVersion in SDK

pranavjain97 · 2026-06-05T19:40:37Z

-  const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion: 2 });
+  const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion });


hmm you wanna check with @mohammadalfaiyazbitgo on this one?

pranavjain97 · 2026-06-05T19:44:41Z

    const doc = await drawKeycardAsync({
      jsPDF,
      QRCode,
-      encrypt: (p: { input: string; password: string }) => this.bitgo.encryptAsync(p),
+      encrypt: (p: { input: string; password: string }) => this.bitgo.encryptAsync({ ...p, encryptionVersion }),


encryptionVersion is missing from drawKeycardAsync's encrypt callback type

encryptionVersion isnt passed from the parameters in this case but Ive changed it to add it to drawKeycardAsync's type and thread it from its callers

mohammadalfaiyazbitgo · 2026-06-05T19:50:57Z

#8955 (comment)

AFAIK no, this module is still in beta.

Ticket: WCN-774

bdesoky force-pushed the WCN-774 branch 2 times, most recently from a712962 to 5576861 Compare June 5, 2026 16:28

bdesoky force-pushed the WCN-774 branch from 5576861 to de9277c Compare June 5, 2026 16:45

bdesoky commented Jun 5, 2026

View reviewed changes

bdesoky marked this pull request as ready for review June 5, 2026 16:54

bdesoky requested review from a team as code owners June 5, 2026 16:54

bdesoky requested review from lokesh-bitgo and rajangarg047 June 5, 2026 16:54

bdesoky force-pushed the WCN-774 branch from de9277c to 0f9bc8f Compare June 5, 2026 17:22

bdesoky commented Jun 5, 2026

View reviewed changes

vinhkhangtieu previously approved these changes Jun 5, 2026

View reviewed changes

pranavjain97 reviewed Jun 5, 2026

View reviewed changes

feat: thread encryptionVersion in SDK

cb174f4

Ticket: WCN-774

bdesoky dismissed vinhkhangtieu’s stale review via cb174f4 June 5, 2026 20:02

bdesoky force-pushed the WCN-774 branch from 0f9bc8f to cb174f4 Compare June 5, 2026 20:02

bdesoky requested a review from pranavjain97 June 5, 2026 20:02

pranavjain97 approved these changes Jun 5, 2026

View reviewed changes

		const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion: 2 });
		const encryptedPrv = await bitgo.encryptAsync({ password: prfPassword, input: privateKey, encryptionVersion });

Conversation

bdesoky commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear-code Bot commented Jun 5, 2026

Uh oh!

bdesoky commented Jun 5, 2026

Uh oh!

github-actions Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review for PR #8955: feat: thread encryptionVersion in SDK

Todo List

Summary

Strengths

Areas for Improvement

Technical Assessment

Files with Excellent Implementation

Recommendation

Uh oh!

bdesoky Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

bdesoky Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

bdesoky Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

bdesoky Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

vinhkhangtieu left a comment

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

pranavjain97 Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

bdesoky Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mohammadalfaiyazbitgo commented Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bdesoky commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 5, 2026 •

edited

Loading

bdesoky Jun 5, 2026 •

edited

Loading